exporter/containerimage: new option: rewrite-timestamp (Apply `SOURCE_DATE_EPOCH` to file timestamps) #4057

AkihiroSuda · 2023-07-24T17:45:16Z

rewrite-timestamp rewrites timestamps in layers for reproducible builds.

When a layer is rewritten, an annotation buildkit/rewritten-timestamp=<INT64> is set to the layer.

Example:

buildctl build
  --frontend dockerfile.v0 \
  --opt build-arg:SOURCE_DATE_EPOCH=${SOURCE_DATE_EPOCH} \
  --output type=oci,dest=dest.tar,rewrite-timestamp=true \
  ...

Alternative to:

[Moved to #4057] Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

With this commit, the following workarounds mentioned in docs/build-repro.md are no longer needed for reproducible builds:

# Limit the timestamp upper bound to SOURCE_DATE_EPOCH.
# Workaround for https://github.com/moby/buildkit/issues/3180
ARG SOURCE_DATE_EPOCH
RUN find $( ls / | grep -E -v "^(dev|mnt|proc|sys)$" ) -newermt "@${SOURCE_DATE_EPOCH}" -writable -xdev | xargs touch --date="@${SOURCE_DATE_EPOCH}" --no-dereference

# Squash the entire stage for resetting the whiteout timestamps.
# Workaround for https://github.com/moby/buildkit/issues/3168
FROM scratch
COPY --from=0 / /

Backup

39c3a49 : Abandoned version, contaminates cache

AkihiroSuda · 2023-08-02T15:14:48Z

@tonistiigi
Does this look better than:

[Moved to #4057] Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

?

tonistiigi · 2023-08-03T12:02:39Z

@AkihiroSuda I think until we figure out how this will work with cache-to/from we need to make sure that if rewrite-epoch is set then these blobs will never be 1) exported with cache, also meaning inline-cache would probably need to be incompatible 2) reused in any subsequent build that doesn't set rewrite-epoch to the same value (this is contrary to the compression behavior atm where existing compression is reused).

AkihiroSuda · 2023-08-07T14:06:27Z

@AkihiroSuda I think until we figure out how this will work with cache-to/from we need to make sure that if rewrite-epoch is set then these blobs will never be 1) exported with cache, also meaning inline-cache would probably need to be incompatible 2) reused in any subsequent build that doesn't set rewrite-epoch to the same value (this is contrary to the compression behavior atm where existing compression is reused).

I guess the entire logic should be just decoupled from cache/, and the layer blobs should be just patched in exporter/containerimage/writer.go:commitDistributionManifest() ?

(So, the epoch will be applied to the base layers as well)

AkihiroSuda · 2023-08-07T17:19:57Z

Updated as above.

The rewrite-epoch option no longer touches the cache.
The blobs are converted right before pushing, in exporter/containerimage/writer.go:commitDistributionManifest().

felixdesouza · 2023-08-25T14:45:37Z

I'm really excited for this to arrive! This will save us a lot of time spent doing the docker save -> strip timestamps -> docker load` pipeline. I had a go earlier today on a toy example, and for the most part it worked great. I did run into one issue however:

If I have this Dockerfile:

FROM alpine:latest
ARG SOURCE_DATE_EPOCH

RUN echo felix > asdf.txt

The last layer is not a valid tar file. Namely because it's small enough to be missing padding. In addition does not have the tar end marker, although that could be an implementation detail of tar (I am on macOS). However, without the rewrite, this works as normal and produces a tar file that's readable by macOS tar, so in a way it is a bit of a regression.

Below is the fq output of the stripped tar that's too small:

Below is the fq output of the same layer but without any epoch rewriting:

I haven't attached, but in the non epoch rewritten tar, there's also the end_marker padding that's 1024 bytes long. This obviously doesn't exist in the epoch rewritten tar given that an individual file within the tar is missing the padding block.

Not entirely sure whether this is the right place for this issue, but just wanted to flag.

AkihiroSuda · 2023-08-25T15:16:33Z

@felixdesouza Thanks, fixed in the latest commit. (the tar writer was not flushed)

felixdesouza · 2023-08-25T15:20:56Z

Ah, beat me to it! I think there's one last flush missing, at the end of the archive, we'll get an io.error.EOF, which is the point in which the archive should be closed to write the padding + footer: https://pkg.go.dev/archive/tar#Writer.Close. But I think we'll also miss that without an error check.

AkihiroSuda · 2023-08-25T19:05:40Z

Should be fixed in the latest commit

wiktor-k · 2023-08-25T19:50:08Z

Great to see this being worked on, thanks!

I wonder one thing: would it be beneficial to also add test cases that'd check if the digest does not change? Having reproducible images is super great but I'm worried some other contributor will break it by accident in the future.

wiktor-k · 2023-08-25T20:42:30Z

Already checked (frontend/dockerfile/dockerfile_test.go)

Excellent, thanks for the confirmation 👍

AkihiroSuda · 2023-09-05T00:38:11Z

@tonistiigi PTAL 🙏

#4057 (comment)

tonistiigi · 2023-09-13T07:15:53Z

cache/blobs.go

@@ -422,7 +423,7 @@ func ensureCompression(ctx context.Context, ref *immutableRef, comp compression.
 		}

 		// Resolve converters
-		layerConvertFunc, err := getConverter(ctx, ref.cm.ContentStore, desc, comp)
+		layerConvertFunc, err := converter.New(ctx, ref.cm.ContentStore, desc, comp)


can be follow-up: This New pattern doesn't work with function anymore and should probably return a struct/interface.

Why not work?

New implies a new instance of a struct.

New implies a new instance of a struct.

I'm not sure.
I thought it can return any.

util/converter/tarconverter/tarconverter.go

util/converter/converter.go

tonistiigi · 2023-09-13T07:40:56Z

exporter/containerimage/export.go

+						// e.unpackImage cannot be used because src ref does not point to the rewritten image
+						err = errors.New("exporter option \"rewrite-epoch\" conflicts with \"unpack\"")
+					} else {
+						err = e.unpackImage(ctx, img, src, session.NewGroup(sessionID))


Should we change this function so that it takes Result[Remote] as parameter. This can be follow-up but I do think we should implement it and not error. Same for push.

Sorry, not sure how it relates to this PR

There are new conditions now for unpack and push, instead of reusing the code-path. And unpack case errors.

Got it, but let me defer that to a follow-up 🙏

exporter/containerimage/writer.go

tonistiigi

More follow-ups: when the layer conversion happens, we should show a progress line about it to the user.

exporter/containerimage/writer.go

Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda · 2023-09-14T02:46:56Z

More follow-ups: when the layer conversion happens, we should show a progress line about it to the user.

done

AkihiroSuda · 2023-09-14T02:48:35Z

BTW, on the second thought, rewrite-epoch may sound weird as English?

rewrite-timestamp might be less confusing?
Or rewrite-layer-timestamp?

AkihiroSuda · 2023-09-14T02:58:07Z

Changed to rewrite-timestamp

(Backup: commit 0265bb7 (rewrite-epoch))

rewrite-timestamp rewrites timestamps in layers for reproducible builds. When a layer is rewritten, an annotation "buildkit/rewritten-timestamp=<INT64>" is set to the layer. Example: ``` buildctl build --frontend dockerfile.v0 \ --opt build-arg:SOURCE_DATE_EPOCH=${SOURCE_DATE_EPOCH} \ --output type=oci,dest=dest.tar,rewrite-timestamp=true \ ... ``` Alternative to PR 3560 Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda added the area/reproducible-builds label Jul 24, 2023

AkihiroSuda mentioned this pull request Jul 24, 2023

[Moved to #4057] Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

Closed

AkihiroSuda force-pushed the rewrite-epoch branch from b32dffa to cabb867 Compare July 24, 2023 17:50

AkihiroSuda changed the title ~~exporter/containerimage: new option: rewrite-epoch~~ exporter/containerimage: new option: rewrite-epoch (Apply SOURCE_DATE_EPOCH to file timestamps) Jul 24, 2023

AkihiroSuda force-pushed the rewrite-epoch branch from cabb867 to de78107 Compare July 24, 2023 18:08

AkihiroSuda added this to the v0.13 milestone Jul 24, 2023

AkihiroSuda force-pushed the rewrite-epoch branch 2 times, most recently from 15e643d to f54de34 Compare July 25, 2023 13:42

AkihiroSuda force-pushed the rewrite-epoch branch from f54de34 to 374410d Compare August 3, 2023 08:49

AkihiroSuda force-pushed the rewrite-epoch branch from 374410d to 39c3a49 Compare August 4, 2023 14:38

AkihiroSuda marked this pull request as draft August 4, 2023 14:39

AkihiroSuda force-pushed the rewrite-epoch branch from 39c3a49 to 01c3d22 Compare August 7, 2023 17:18

AkihiroSuda marked this pull request as ready for review August 7, 2023 17:19

AkihiroSuda force-pushed the rewrite-epoch branch 3 times, most recently from 46f3787 to 4963e73 Compare August 7, 2023 17:39

AkihiroSuda requested a review from tonistiigi August 10, 2023 00:30

AkihiroSuda force-pushed the rewrite-epoch branch from 4963e73 to a3847fb Compare August 25, 2023 15:16

AkihiroSuda force-pushed the rewrite-epoch branch 2 times, most recently from fdde4db to 1ed7a11 Compare August 25, 2023 19:03

AkihiroSuda force-pushed the rewrite-epoch branch from 1ed7a11 to 52fb34e Compare August 25, 2023 19:22

tonistiigi reviewed Sep 13, 2023

View reviewed changes

AkihiroSuda force-pushed the rewrite-epoch branch from 52fb34e to 852e57b Compare September 13, 2023 08:40

AkihiroSuda marked this pull request as draft September 13, 2023 08:48

AkihiroSuda force-pushed the rewrite-epoch branch from 852e57b to 8579128 Compare September 13, 2023 09:03

AkihiroSuda marked this pull request as ready for review September 13, 2023 09:04

AkihiroSuda force-pushed the rewrite-epoch branch from 8579128 to e979a33 Compare September 13, 2023 09:04

tonistiigi approved these changes Sep 13, 2023

View reviewed changes

exporter/containerimage/writer.go Outdated Show resolved Hide resolved

exporter/containerimage/writer.go Show resolved Hide resolved

Move cache/converter.go to util/converter/converter.go

a36dbcf

Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda force-pushed the rewrite-epoch branch from e979a33 to 0265bb7 Compare September 14, 2023 02:46

AkihiroSuda force-pushed the rewrite-epoch branch from 0265bb7 to 8fcc411 Compare September 14, 2023 02:57

AkihiroSuda changed the title ~~exporter/containerimage: new option: rewrite-epoch (Apply SOURCE_DATE_EPOCH to file timestamps)~~ exporter/containerimage: new option: rewrite-timestamp (Apply SOURCE_DATE_EPOCH to file timestamps) Sep 14, 2023

AkihiroSuda force-pushed the rewrite-epoch branch 3 times, most recently from 6a8e2e3 to 1b59dfc Compare September 14, 2023 03:01

AkihiroSuda force-pushed the rewrite-epoch branch from 1b59dfc to 2f8292c Compare September 14, 2023 03:12

tonistiigi approved these changes Sep 14, 2023

View reviewed changes

tonistiigi merged commit 915d245 into moby:master Sep 14, 2023

crazy-max added the needs/docs-follow-up label Sep 14, 2023

MajorBreakfast mentioned this pull request Mar 26, 2024

docker build does not create reproducible output (even with fixed SOURCE_DATE_EPOCH) moby/moby#43124

Open

ptoman-cisco mentioned this pull request Jun 3, 2024

rewrite-timestamp with type=docker or type/image #4986

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exporter/containerimage: new option: rewrite-timestamp (Apply `SOURCE_DATE_EPOCH` to file timestamps) #4057

exporter/containerimage: new option: rewrite-timestamp (Apply `SOURCE_DATE_EPOCH` to file timestamps) #4057

AkihiroSuda commented Jul 24, 2023 •

edited

Loading

AkihiroSuda commented Aug 2, 2023

tonistiigi commented Aug 3, 2023

AkihiroSuda commented Aug 7, 2023 •

edited

Loading

AkihiroSuda commented Aug 7, 2023 •

edited

Loading

felixdesouza commented Aug 25, 2023

AkihiroSuda commented Aug 25, 2023

felixdesouza commented Aug 25, 2023

AkihiroSuda commented Aug 25, 2023

wiktor-k commented Aug 25, 2023

wiktor-k commented Aug 25, 2023

AkihiroSuda commented Sep 5, 2023

tonistiigi Sep 13, 2023

AkihiroSuda Sep 13, 2023

tonistiigi Sep 13, 2023

AkihiroSuda Sep 13, 2023

tonistiigi Sep 13, 2023

AkihiroSuda Sep 13, 2023

tonistiigi Sep 13, 2023

AkihiroSuda Sep 13, 2023

tonistiigi left a comment

AkihiroSuda commented Sep 14, 2023

AkihiroSuda commented Sep 14, 2023

AkihiroSuda commented Sep 14, 2023

exporter/containerimage: new option: rewrite-timestamp (Apply SOURCE_DATE_EPOCH to file timestamps) #4057

exporter/containerimage: new option: rewrite-timestamp (Apply SOURCE_DATE_EPOCH to file timestamps) #4057

Conversation

AkihiroSuda commented Jul 24, 2023 • edited Loading

AkihiroSuda commented Aug 2, 2023

tonistiigi commented Aug 3, 2023

AkihiroSuda commented Aug 7, 2023 • edited Loading

AkihiroSuda commented Aug 7, 2023 • edited Loading

felixdesouza commented Aug 25, 2023

AkihiroSuda commented Aug 25, 2023

felixdesouza commented Aug 25, 2023

AkihiroSuda commented Aug 25, 2023

wiktor-k commented Aug 25, 2023

wiktor-k commented Aug 25, 2023

AkihiroSuda commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonistiigi left a comment

Choose a reason for hiding this comment

AkihiroSuda commented Sep 14, 2023

AkihiroSuda commented Sep 14, 2023

AkihiroSuda commented Sep 14, 2023

exporter/containerimage: new option: rewrite-timestamp (Apply `SOURCE_DATE_EPOCH` to file timestamps) #4057

exporter/containerimage: new option: rewrite-timestamp (Apply `SOURCE_DATE_EPOCH` to file timestamps) #4057

AkihiroSuda commented Jul 24, 2023 •

edited

Loading

AkihiroSuda commented Aug 7, 2023 •

edited

Loading

AkihiroSuda commented Aug 7, 2023 •

edited

Loading